TAJO-939: Refactoring the column resolver in LogicalPlan#71
Conversation
First refactoring of resolver.
Refactored many of createEval to use NameResolver.
Refactored CreateEval() except for join.
Completely extracted the column resolver from LogicalPlan.
There was a problem hiding this comment.
It would be better use the above 'projectTargetExprs' variable.
There was a problem hiding this comment.
Thank you for your suggestion. I'll do so.
|
Hi Hyunsik, I have a question. |
|
Thank you for the quick review. Unfortunately, I couldn't find any rule in literatures and SQL standards. I expect that it is not specified in SQL standards because commercial DBMSs have different behaviors. So, I analyzed the behavior of PostgreSQL, and then I made it. |
|
Thanks for your reply. |
|
Please see the following comments. I've just added the explanation about the motivation, all modes, and an example. I hope that it's helpful for your understanding. |
|
Thanks! |
There was a problem hiding this comment.
In setRawTargets(), raw expressions are found instead of references. So, it seems that resolving level should be set as RELS_ONLY. If I misunderstand, please let me know. Thanks.
There was a problem hiding this comment.
name references in select list can be only actual column names. They are addressed in the manner at https://github.com/apache/tajo/pull/71/files#diff-c11c2244fea624cb7b08f6906d554cd4R470.
In contrast, setRawTargets() in visitProjection should deal with actual fields as well as temporal fields generated by common subexpression elimination. So, it should use RELS_AND_SUBEXPRS.
|
Hi Hyunsik. I left some questions. |
Conflicts: tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlanPreprocessor.java tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java
There was a problem hiding this comment.
As you commented, fields in LIMIT are resolved in the REL_ONLY mode. Please fix it.
|
@jihoonson I really appreciate your detailed review. I rebased the branch against the latest revision, and I reflected your comment. Please help to review it again. Thank you! |
|
@jihoonson Join requires more complex conditions. So, I need to clean up the name resolving logic for join condition, and I need more time for this job. If you are Ok, I'll do it in another Jira. |
|
Rebased. |
|
Thanks for your reply! |
|
And please create a Jira issue for name resolution of join condition. |
|
Thank you very much. I'll create the jira. |
|
I've made the jira to clean up the name resolving logic for join condition. |
ZEPPELIN-121 Fix handling escpae char in erb evaluator
2 participants
See the description in TAJO-939 (https://issues.apache.org/jira/browse/TAJO-939).
I fixed the problem mentioned in the description. Also, I refactored the name resolver as follows: